Emit Self in generated code for clippy::use_self compliance#15
Merged
iainmcgin merged 6 commits intoanthropics:mainfrom Mar 30, 2026
Merged
Emit Self in generated code for clippy::use_self compliance#15iainmcgin merged 6 commits intoanthropics:mainfrom
iainmcgin merged 6 commits intoanthropics:mainfrom
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
Contributor
Author
|
I have read the CLA Document and I hereby sign the CLA |
The clippy auto-fix in the original commit touched generated .rs files directly, which check-generated-code CI would revert. This updates the two quote! emission sites (impl_message.rs, view.rs) so codegen actually produces Self::default() in DefaultInstance/DefaultViewInstance impls, then regenerates. A few of clippy's other generated-file substitutions (Vec<Self> for self-referential struct fields, Self:: in oneof From impls) are normalized back by regeneration — those would need separate codegen changes and are out of scope here. The hand-written source changes from the original commit all remain.
Covers the remaining clippy::use_self sites in generated oneof code: - Self::Variant(...) inside impl From<T> for OneofEnum - Self::Some(...) inside impl From<T> for Option<OneofEnum> The one case left unaddressed is self-referential struct fields (DescriptorProto.nested_type: Vec<DescriptorProto> could be Vec<Self>). Emitting Self there requires comparing resolved field types against the current message ident during struct generation — invasive for a single beneficiary. Deferred.
Add FieldInfo.struct_field_type alongside rust_type. The struct-field declaration uses struct_field_type (with Self substituted for self-referential message types); rust_type stays concrete for the serde-deserialize codegen, which runs inside impl Visitor for _V where Self would bind to the wrong type. The substitution compares field.type_name against the current message's dotted FQN — no token-level inspection needed. Exercised by DescriptorProto.nested_type: now Vec<Self>. (conformance/Cargo.lock picked up buffa-descriptor from the rebase onto main — that crate split landed in anthropics#8.)
Mirror of the owned-struct change in the previous commit, applied in view_struct_field. Inside struct FooView<'a>, Self expands to FooView<'a> with the lifetime applied, so RepeatedView<'a, Self> is exactly equivalent to the concrete form. Found by running clippy::use_self on the googleapis stress output — the only two remaining warnings were HttpRuleView.additional_bindings and DeclTypeView.type_params. With this commit the entire googleapis corpus (47 files, ~65K lines) is clippy::use_self clean.
argo clippy --allow-dirty --fix -- -Wclippy::use_self
Collaborator
|
Thanks for the contribution! I updated code generation to emit |
iainmcgin
approved these changes
Mar 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Originally a
cargo clippy --fix -- -Wclippy::use_selfrun on hand-written source. Expanded to update the codegen emission sites so generated output is alsouse_self-clean — otherwisecheck-generated-codeCI would revert the clippy fixes on the next regeneration.Verified by running
clippy -W clippy::use_selfon the googleapis stress test output (47 files, ~65K lines): zero warnings.Codegen changes
b1c69c1impl_message.rs,view.rsDefaultInstance::default_instance→Self::default()b3e7faaoneof.rsimpl From<T> for OneofEnum→Self::Variant(...),Self::Some(...)79e2a19message.rsVec<Self>/MessageField<Self>824942fview.rsRepeatedView<'a, Self>Design note:
FieldInfo.struct_field_typeSelf-reference detection for struct fields compares the field's
type_name(proto FQN) against the current message's FQN — no token inspection needed. The result is stored as a separateFieldInfo.struct_field_typealongside the concreterust_type, becauserust_typeis also consumed by serde-deserialize codegen running insideimpl Visitor for _V, whereSelfwould bind to_Vinstead of the message (caught by the conformance build).Edge cases validated
google.api.expr.v1alpha1.Type— self-ref inside a oneof. Variant declaration correctly usessuper::Type;Fromimpls correctly useSelf::TypeandSelf::Some. ThreeSelfscopes, all correct.TestAllRequiredTypesProto2.recursive_message(conformance) — JSON-deserialize Visitor scoping.